🌱 Deduplicate metrics service lookup and port-forward in e2e steps#2710
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in e2e metrics/TLS step helpers by centralizing metrics service discovery and kubectl port-forward setup/cleanup.
Changes:
- Extracts shared helpers for metrics service lookup, metrics port selection, random port allocation, and port-forward startup.
- Rewrites TLS metrics port-forwarding and metrics request steps to use the shared helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/e2e/steps/tls_steps.go |
Adds shared metrics service and port-forward helpers; updates TLS metrics forwarding to use them. |
test/e2e/steps/steps.go |
Updates metrics request flow to use shared service lookup and port-forward helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sc.metricsResponse = make(map[string]string) | ||
| for _, p := range pods { | ||
| port, err := randomAvailablePort() | ||
| addr, cleanup, err := portForward(p.Namespace, fmt.Sprintf("pod/%s", p.Name), mPort) |
|
/lgtm |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2710 +/- ##
==========================================
+ Coverage 68.12% 68.14% +0.01%
==========================================
Files 145 145
Lines 10698 10698
==========================================
+ Hits 7288 7290 +2
+ Misses 2880 2879 -1
+ Partials 530 529 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SendMetricsRequest and withMetricsPortForward duplicated service endpoint discovery and port-forward lifecycle code. Extract shared helpers (getMetricsService, metricsPort, portForward) and rewrite both callers to use them. Port-forward cleanup is deferred to avoid process leaks if waitFor panics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fgiudici, perdasilva, rashmigottipati The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
openshift-merge-bot
Bot
merged commit 4988d38
into
operator-framework:main
Description
SendMetricsRequest(insteps.go) andwithMetricsPortForward(intls_steps.go) duplicated two chunks of logic:corev1.Service, and looped its ports looking for"metrics".randomAvailablePort(), built akubectl port-forwardcommand, started it, and deferred kill+wait cleanup.This PR extracts shared helpers and rewrites both callers to use them:
getMetricsService(component) (*corev1.Service, error)— returns the full Service object (namespace available viasvc.Namespace)metricsPort(svc) (int32, error)— extracts the metrics port from a serviceportForward(ns, target, remotePort) (addr, cleanup, error)— reusable port-forward lifecyclerandomAvailablePortmoved fromsteps.gototls_steps.goalongsideportForwardReviewer Checklist